feat: Switch to culori color library by camdecoster · Pull Request #7962 · plotly/plotly.js

camdecoster · 2026-08-13T16:59:58Z

Description

Switch to the culori color processing library.

Closes #7961.

Changes

Remove color library
Add culori library
Update color functions per library change
Update call sites
Update and add tests

Screenshots

color_syntax_formats mock results

Before	After

Testing

Check CI results
Review the color_syntax_formats mock and try changing some of the color specifiers to valid/invalid values
Try entering some of the color specifiers from the mock into an element through browser devtools and see how it compares to the after results

Notes

This library bumps up the bundle size a bit (+17KB gzipped) but it's worth the added CSS 4 compatibility
We can mitigate this in the future by switching to ESM for this file (so it can be tree-shaken)
I removed the color-normalize dependency in favor of managing that internally

… check

emilykl · 2026-08-14T18:35:07Z

+
+                    nodes.each(function (_, i) {
+                        const { fill } = this.style;
+                        const other = Color.equals(fill, Color.background) ? Color.defaultLine : Color.background;


Suggested change

const other = Color.equals(fill, Color.background) ? Color.defaultLine : Color.background;

// Ensure text fill color matches either Color.background or Color.defaultLine

expect([Color.background, Color.defaultLine]).toContain(fill);

const other = Color.equals(fill, Color.background) ? Color.defaultLine : Color.background;

emilykl · 2026-08-14T18:42:50Z

+        // A translucent axis color is what exercises the rule: the channel weight
+        // scales by the alpha difference, so mixing toward an opaque background
+        // moves the channels less than a plain interpolation would.


Suggested change

// A translucent axis color is what exercises the rule: the channel weight

// scales by the alpha difference, so mixing toward an opaque background

// moves the channels less than a plain interpolation would.

// axis.gridcolor is determined by mixing the axis color with the paper and plot background colors.

// If the axis color contains an alpha channel, its weight should scale by the alpha value, so that

// more-transparent colors are weighted less.

emilykl · 2026-08-14T18:43:17Z

+            };
+
+            supplyLayoutDefaults(layoutIn, layoutOut, fullData);
+            expect(layoutOut.xaxis.gridcolor).toEqual('rgba(255, 247, 0, 0.95)');


Maybe add a comment here stating what gridcolor we would expect if the weight was not scaled by alpha?

emilykl · 2026-08-14T18:44:46Z

+            Color.fill({ style: (o) => seen.push(o) }, undefined); // Mock the selection to track it's call
+


Suggested change

Color.fill({ style: (o) => seen.push(o) }, undefined); // Mock the selection to track it's call

Color.fill({ style: (o) => seen.push(o) }, undefined);

emilykl · 2026-08-14T18:45:13Z

+            Color.stroke({ style: (o) => seen.push(o) }, undefined); // Mock the selection to track it's call
+


Suggested change

Color.stroke({ style: (o) => seen.push(o) }, undefined); // Mock the selection to track it's call

Color.stroke({ style: (o) => seen.push(o) }, undefined);

emilykl · 2026-08-14T18:48:31Z

+                const other = Color.equals(picked, Color.background) ? Color.defaultLine : Color.background;
+


Suggested change

const other = Color.equals(picked, Color.background) ? Color.defaultLine : Color.background;

// Ensure selected color matches either Color.background or Color.defaultLine

expect([Color.background, Color.defaultLine]).toContain(picked);

const other = Color.equals(picked, Color.background) ? Color.defaultLine : Color.background;

emilykl · 2026-08-14T18:51:15Z

+        // Drawing code needs the alpha of a color it is about to paint, which is
+        // not the same question `opacity` answers. A color that is simply unset
+        // still gets painted, so it resolves to opaque black.


Suggested change

// Drawing code needs the alpha of a color it is about to paint, which is

// not the same question `opacity` answers. A color that is simply unset

// still gets painted, so it resolves to opaque black.

emilykl · 2026-08-14T18:52:58Z

+        // Drawing code needs the alpha of a color it is about to paint, which is
+        // not the same question `opacity` answers. A color that is simply unset
+        // still gets painted, so it resolves to opaque black.
+        it('treats a missing color as opaque black, without warning', () => {


The description implies that the test checks all channels of the color, but it only checks the alpha. Either the description or the test should be updated (I don't think it checks for a warning either, although maybe our test infrastructure does surface warnings).

Same for the next test.

Suggested change

it('treats a missing color as opaque black, without warning', () => {

it('treats a missing color as opaque', () => {

emilykl · 2026-08-14T18:56:33Z

+        });
+    });
+
+    describe('parse', () => {


Honestly all of these parse tests could probably be rolled into other sections

emilykl · 2026-08-14T18:57:37Z

+            BAD.forEach((v) => expect(Color.isValid(v)).toBe(false));
+        });
+
+        // Null channels used to reach the WebGL buffers through this path.


Suggested change

// Null channels used to reach the WebGL buffers through this path.

emilykl · 2026-08-14T19:21:45Z

+        it('sets alpha', () => {
+            expect(Color.addOpacity('red', 0.5)).toBe('rgba(255, 0, 0, 0.5)');
+            expect(Color.addOpacity('rgba(255, 0, 0, 0.5)', 1)).toBe('rgb(255, 0, 0)');
+            expect(Color.addOpacity('red', 2)).toBe('rgb(255, 0, 0)');


No opacity is added here: is that because 2 is an invalid value so the function does nothing, or because a ceiling is applied so that 2 becomes 1?

emilykl · 2026-08-14T19:25:10Z

+            expect(fills.length).toBe(5);
+            expect(fills.every((f) => f === 'rgb(0, 0, 0)')).toBe(false, 'all points black');
+            expect(new Set(fills).size).toBeGreaterThan(1, 'every point the same color');
+            fills.forEach((f) => expect(f).toMatch(/^rgba?\(/, `not a usable color: ${f}`));


Instead of all this, could you just expect(fills).toBe(...) and copy the actual array expected for the Viridis colorscale?

emilykl · 2026-08-14T19:58:22Z

+ * Parse a color specifier, falling back to opaque black.
+ *
+ * A missing color falls back quietly, because it means the caller left the
+ * attribute unset rather than gave a bad value. Callers that treat a missing
+ * color as nothing to paint test for it themselves, as `opacity` does.
 *
 * @param {*} cstr - color specifier
- * @return {Color} color object
+ * @param {Boolean} [silent] - skip the warning, for callers that run per data point


Suggested change

* Parse a color specifier, falling back to opaque black.

*

* A missing color falls back quietly, because it means the caller left the

* attribute unset rather than gave a bad value. Callers that treat a missing

* color as nothing to paint test for it themselves, as `opacity` does.

*

* @param {*} cstr - color specifier

* @return {Color} color object

* @param {Boolean} [silent] - skip the warning, for callers that run per data point

* Parse a color specifier string and return it as a culori rgb color object.

* If the input is not a string or cannot be parsed, fall back to opaque black (#fff).

*

* @param {String} cstr - color specifier

* @param {Boolean} [silent] - if true, do not emit a warning for un-parseable colors

emilykl · 2026-08-14T20:00:52Z

 * Convert any color specifier to a normalized `rgb(r, g, b)` string.
+ * Force alpha to 1 so that it gets dropped in the result.


Since this function calls parse(), that means any invalid input will return rgb(0, 0, 0), right?

Might be worth putting that info in the docstring.

emilykl · 2026-08-14T20:01:31Z

 * Return the alpha channel of a color (0 if falsy).
 *
 * @param {*} cstr - color specifier
 * @return {Number}


What is the range for the number, is it [0, 1]?

emilykl · 2026-08-14T20:18:40Z

 * @param {*} cstr - color specifier
- * @return {Color} color object
+ * @param {Boolean} [silent] - skip the warning, for callers that run per data point
+ * @return {Object} culori rgb color


A culori rgb object is just an object that looks like

{ mode: 'rgb', r: _, g: _, b: _, alpha: _ }

right? Maybe add that info to the docstring since parse() is used everywhere.

emilykl · 2026-08-14T20:21:45Z

 /**
 * Convert a color specifier to a 4-element `[r, g, b, a]` representation.
- * Accepts strings, numeric float arrays (`[0, 1]`), or uint8 arrays (`[0, 255]`).
+ * Falls back to opaque black rather than null: WebGL paths index the result.


Suggested change

* Falls back to opaque black rather than null: WebGL paths index the result.

* Returns opaque black ([0, 0, 0, 0]) if color specifier is invalid.

emilykl · 2026-08-14T20:24:49Z

 * @param {*} cstr - color specifier
- * @param {Number} op - opacity in [0, 1]
- * @return {String}
+ * @param {Number} op - opacity in [0, 1], clipped to that range


clipped to that range

Unclear — does this mean the input must be clipped to [0, 1] before passing to this function, or that if it's outside [0, 1] this function will clip it?

In any case, I see the clipping for values above 1, but what happens when the value is below 0?

emilykl · 2026-08-14T20:31:04Z


 /**
 * Combine two colors into one apparent color by compositing `front` over `back`.
 * If `back` is missing or transparent, the module `background` is assumed behind it.


Suggested change

* If `back` is missing or transparent, the module `background` is assumed behind it.

* If `back` is missing, the module `background` is assumed behind it.

emilykl · 2026-08-14T20:57:05Z

 * @param {*} cstr - color specifier
 * @param {Number} delta - lightness shift in HSL percentage points
- * @return {Color} adjusted color object
+ * @return {String} resulting color string


Suggested change

* @return {String} resulting color string

* @return {String} resulting color string as rgb

emilykl · 2026-08-14T21:13:54Z

+    const newColor = isDark(cstr)
        ? lightAmount
-            ? adjustLightness(c, lightAmount)
-            : color(background)
+            ? adjustLightness(cstr, lightAmount)
+            : background
        : darkAmount
-          ? adjustLightness(c, -darkAmount)
-          : color(defaultLine);
+          ? adjustLightness(cstr, -darkAmount)
+          : defaultLine;

-    return newColor.rgb().string();
+    return formatRgb(parse(newColor));


If newColor is the result of adjustLightness(), then it's already a formatted rgb string and the extra round-trip through formatRgb(parse(...)) is unnecessary, right?

Suggested change

const newColor = isDark(cstr)

? lightAmount

? adjustLightness(c, lightAmount)

: color(background)

? adjustLightness(cstr, lightAmount)

: background

: darkAmount

? adjustLightness(c, -darkAmount)

: color(defaultLine);

? adjustLightness(cstr, -darkAmount)

: defaultLine;

return newColor.rgb().string();

return formatRgb(parse(newColor));

const newColor = isDark(cstr)

? lightAmount

? adjustLightness(cstr, lightAmount)

: formatRgb(parse(background));

: darkAmount

? adjustLightness(cstr, -darkAmount)

: formatRgb(parse(defaultLine));

return newColor;

Honestly it would probably make sense to convert the constants to RGB just once at the top of the file and reuse them.

const backgroundRGB = formatRgb(parse(background)) const defaultLineRGB = formatRgb(parse(defaultLine))

emilykl · 2026-08-14T21:19:50Z

+ * A missing color paints opaque black. Shapes and annotations leave
+ * `line.color` unset when the user gives none, and the outline still has to
+ * show. Use `opacity` instead when a missing color means "nothing to paint".


Suggested change

* A missing color paints opaque black. Shapes and annotations leave

* `line.color` unset when the user gives none, and the outline still has to

* show. Use `opacity` instead when a missing color means "nothing to paint".

* A missing or invalid color specifier applies opaque black.

emilykl · 2026-08-14T21:21:01Z

 /**
 * Apply `fill` and `fill-opacity` styles to a D3 selection.
 *
+ * A missing color paints opaque black, the same as `stroke`.


Suggested change

* A missing color paints opaque black, the same as `stroke`.

* A missing or invalid color specifier applies opaque black.

emilykl · 2026-08-14T21:33:37Z

+    const w = 2 * p - 1;
+    const w2 = ((w * d === -1 ? w : (w + d) / (1 + w * d)) + 1) / 2;


I'm sure this is fine, but do you know where these equations come from?

emilykl · 2026-08-14T21:48:17Z

 };

+/**
+ * Convert any color specifier to an `rgb(...)` or `rgba(...)` string,


Probably all of these functions should specify that passing an invalid color specifier returns black.

emilykl · 2026-08-14T21:50:44Z

+ * Channels as `[r, g, b, a]`, with `r`/`g`/`b` in [0, 255] and `a` in [0, 1].
+ * An array rather than an object so callers cannot depend on the color library's
+ * shape. Unrounded, since callers do further arithmetic.


Suggested change

* Channels as `[r, g, b, a]`, with `r`/`g`/`b` in [0, 255] and `a` in [0, 1].

* An array rather than an object so callers cannot depend on the color library's

* shape. Unrounded, since callers do further arithmetic.

* Returns the given color specifier as an `[r, g, b, a]` array,

* with `r`/`g`/`b` in [0, 255] and `a` in [0, 1].

emilykl · 2026-08-14T21:53:17Z

    }
+    // `toRgb` omits alpha when it's 1; make sure it's added since we expect it
+    c.alpha ??= 1;
+


Is there any reason not to clip each of the r, g, b values before returning the color object?

emilykl · 2026-08-14T22:00:00Z

-    var colorOut = rgba(colorIn);
+    // A per-point color may be raw channels rather than a color string, which
+    // `Color.isValid` rejects but `Color.normalize` handles.
+    if (!isArrayOrTypedArray(colorIn) && !Color.isValid(colorIn)) return colorDfltRgba;


Would it make sense to use Color.isChannelArray() here?

Suggested change

if (!isArrayOrTypedArray(colorIn) && !Color.isValid(colorIn)) return colorDfltRgba;

if (!Color.isChannelArray(colorIn) && !Color.isValid(colorIn)) return colorDfltRgba;

But actually this logic feels like a code smell. How about adding a parameter arrayAllowed to Color.isValid() so that you could write

if (!Color.isValid(colorIn, true)) return colorDfltRgba;

emilykl

Left a bunch of comments, but nothing blocking 🌈

camdecoster added 17 commits August 12, 2026 10:46

Switch to culori color library

d46b3f4

Update color functions per the new API

3525cc8

Update color function call sites

c87fa57

Add text contrast test helper

927ab57

Add/update tests

3cc82a3

Update color syntax mock

7d93b21

Add comments about renaming

7c85245

Add array support to Color.normalize

fbc644e

Default to black for undefined values in Color.stroke, Color.fill

7ab0f92

Delete obsolete mock

a3e909c

Revert changes to fill/stroke, rename parse, switch to previous alpha…

765ffca

… check

Add more tests

9c0e4b8

Delete another obsolete mock

ec45487

Update mock to show whole legend

4992e04

Remove dead code

1c23467

Update baseline images

0403993

Add draftlog

13373c7

camdecoster marked this pull request as ready for review August 14, 2026 17:35

emilykl reviewed Aug 14, 2026

View reviewed changes

emilykl approved these changes Aug 14, 2026

View reviewed changes

		Color.fill({ style: (o) => seen.push(o) }, undefined); // Mock the selection to track it's call

		Color.stroke({ style: (o) => seen.push(o) }, undefined); // Mock the selection to track it's call

		const other = Color.equals(picked, Color.background) ? Color.defaultLine : Color.background;

	// Drawing code needs the alpha of a color it is about to paint, which is
	// not the same question `opacity` answers. A color that is simply unset
	// still gets painted, so it resolves to opaque black.

	it('treats a missing color as opaque black, without warning', () => {
	it('treats a missing color as opaque', () => {

		* Convert any color specifier to a normalized `rgb(r, g, b)` string.
		* Force alpha to 1 so that it gets dropped in the result.

	* Falls back to opaque black rather than null: WebGL paths index the result.
	* Returns opaque black ([0, 0, 0, 0]) if color specifier is invalid.

	* If `back` is missing or transparent, the module `background` is assumed behind it.
	* If `back` is missing, the module `background` is assumed behind it.

	* @return {String} resulting color string
	* @return {String} resulting color string as rgb

	* A missing color paints opaque black, the same as `stroke`.
	* A missing or invalid color specifier applies opaque black.

		const w = 2 * p - 1;
		const w2 = ((w * d === -1 ? w : (w + d) / (1 + w * d)) + 1) / 2;

	if (!isArrayOrTypedArray(colorIn) && !Color.isValid(colorIn)) return colorDfltRgba;
	if (!Color.isChannelArray(colorIn) && !Color.isValid(colorIn)) return colorDfltRgba;

Uh oh!

Uh oh!

Conversation

camdecoster commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Screenshots

Testing

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilykl Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilykl Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilykl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

camdecoster commented Aug 13, 2026 •

edited

Loading

emilykl Aug 14, 2026 •

edited

Loading

emilykl Aug 14, 2026 •

edited

Loading